Skip to content

Conversation

@lntue
Copy link
Contributor

@lntue lntue commented Oct 31, 2024

No description provided.

@llvmbot llvmbot added the libc label Oct 31, 2024
@lntue lntue changed the title [libc] Fix memory leak in MPFR cospif with MPFR pre 4.2. [libc] Fix memory leak in MPFRWrapper cospif with MPFR pre 4.2. Oct 31, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/114415.diff

1 Files Affected:

  • (modified) libc/utils/MPFRWrapper/MPFRUtils.cpp (+2-1)
diff --git a/libc/utils/MPFRWrapper/MPFRUtils.cpp b/libc/utils/MPFRWrapper/MPFRUtils.cpp
index 60e4abadb5e3c8..4fa6d2f9291993 100644
--- a/libc/utils/MPFRWrapper/MPFRUtils.cpp
+++ b/libc/utils/MPFRWrapper/MPFRUtils.cpp
@@ -262,6 +262,7 @@ class MPFRNumber {
 
       int d = mpz_tstbit(integer, 0);
       mpfr_set_si(result.value, d ? -1 : 1, mpfr_rounding);
+      mpz_clear(integer);
       return result;
     }
 
@@ -271,7 +272,7 @@ class MPFRNumber {
     mpfr_cos(result.value, value_pi.value, mpfr_rounding);
 
     return result;
-#endif
+// #endif
   }
 
   MPFRNumber erf() const {

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this was fixed in a newer version of MPFR, consider adding a link to the commit in the PR description, and a comment to the sources if this statement should be removed at some point.

@lntue
Copy link
Contributor Author

lntue commented Oct 31, 2024

If this was fixed in a newer version of MPFR, consider adding a link to the commit in the PR description, and a comment to the sources if this statement should be removed at some point.

Unfortunately, this is a bug in our wrapper, not on MPFR side. With newer MPFR, we just call the cospi function directly and there is no bug there.

@lntue lntue merged commit 296a9ba into llvm:main Oct 31, 2024
4 of 5 checks passed
@lntue lntue deleted the cospif branch October 31, 2024 15:19
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this pull request Nov 3, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants